Skip to content

fix: report a close or cancel during a wait as OperationStateError - #609

Open
vuanhphung wants to merge 2 commits into
mainfrom
fix/close-during-wait-reports-closed
Open

vuanhphung wants to merge 2 commits into
mainfrom
fix/close-during-wait-reports-closed

Conversation

@vuanhphung

@vuanhphung vuanhphung commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Closing a session while one of its queries is running should make the code waiting on that query (fetchAll(), fetchChunk(), finished()) fail with a closed error. On Thrift it often fails with the transport error instead:

THTTPException: Received a response with a bad HTTP status code: 409: ABORTED: A concurrent GetOperationStatus request updated the command. Please retry.

session.close() sends CloseOperation while the wait's GetOperationStatus is still in flight. The server then answers that poll with HTTP 409 about two times in three, and DBSQLOperation passes the error through, even though the client itself closed the operation. The "Please retry" text points callers the wrong way. The databricks-driver-test CONCURRENT-004 test (databricks/databricks-driver-test#2082) reproduced this in 5 of 7 live Thrift runs against main (the proxy recorded the 409 on the poll). In the other runs the poll got HTTP 200 and the driver already reported The operation was closed by a client. A cancel at the same point already gets CANCELED_STATE back, and SEA already reported a closed error.

DBSQLOperation now keeps the latest close or cancel request. When a wait or fetch fails with anything other than an OperationStateError, it waits for that request to settle. If the close or cancel succeeded, it reports OperationStateError CLOSED or CANCELED. If it failed, or there was none, it rethrows the original error.

Verification:

  • New unit tests in tests/unit/DBSQLOperation.test.ts. Three fail without the change, and three cover the cases that must keep the original error, including a close that fails after the poll did.
  • npm test (1757 passing), npm run lint, npm run type-check and prettier are clean.
  • With this build, the driver-test CONCURRENT-004 tripwire XPASSed in all 5 Thrift runs that hit the 409. CONCURRENT-004 on SEA and CONCURRENT-005 on both backends still pass.

This pull request and its description were written by Isaac.

Closing or cancelling an operation while fetchAll(), fetchChunk() or
finished() waits on it could reject the wait with the server's answer
for the released statement. On Thrift, closing the session while a query
runs makes the in-flight GetOperationStatus return HTTP 409 ("A concurrent
GetOperationStatus request updated the command. Please retry."), which
surfaced as a THTTPException. DBSQLOperation now records a close or cancel
from the moment it is requested and reports such failures as
OperationStateError CLOSED or CANCELED.

Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 1 Low

Looks good overall. The new close/cancel flags fix the 409 case as described, and the tests cover the main paths. One low concern: if the in-flight request fails before a close or cancel that then fails, the waiter is told CLOSED/CANCELED while the operation stays open, and the original error is dropped without a log. That case isn't tested.

Comment thread lib/DBSQLOperation.ts Resolved
A status poll or result fetch could fail while the close or cancel that
caused it was still pending. The waiter was then told CLOSED or CANCELED
even if that request went on to fail, leaving the operation open and
dropping the original error. Keep the latest close/cancel request and
await it before choosing the error: report the client's action only if
it succeeded, otherwise rethrow the original error.

Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No issues identified by the review bot.

This branch was previously deployed

1 inactive deployment
azure-prod — f12a671f Deployed Nov 13, 2026 by vuanhphung via e2e-test (26.12.1, 26_12_1) #2007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant